Improved on uxr_close_tcp_platform() to avoid potentially losing bytes in flight. #97
Conversation
|
Build status:
|
|
The error was "ERROR: Error fetching remote repo 'origin' |
sp193
force-pushed
the
graceful_shutdown
branch
from
d8145c4
to
ff4aa80
Compare
|
Build status:
|
| { | ||
| /* Synchronize the stream with the agent, to avoid losing bytes currently in flight. */ | ||
| shutdown(platform->poll_fd.fd, SHUT_WR); | ||
| int poll_rv = poll(&platform->poll_fd, 1, 10000); |
There was a problem hiding this comment.
Maybe we can define the 10000 ms. Why does it need to polling so long? Did you tested it or it is just a approximation?
There was a problem hiding this comment.
It is an approximation. You are right to say that computers would probably not need so much time, but those are under ideal conditions (no network problems, host not overloaded etc). On the other hand, if XRCE Agent encountered the socket write closure, it should follow up very soon. I think it only has to be long enough to cater for the possibility that the Agent has hung or has dropped out of contact, while not blocking the client for a needlessly long period of time.
| int poll_rv = poll(&platform->poll_fd, 1, 10000); | ||
| if (0 < poll_rv) | ||
| { | ||
| char dummy; |
There was a problem hiding this comment.
Maybe we can change it by uint8_t[64] to increase performance?
| bool rv = true; | ||
|
|
||
| if (INVALID_SOCKET != platform->poll_fd.fd) | ||
| { | ||
| /* Synchronize the stream with the agent, to avoid losing bytes currently in flight. */ | ||
| shutdown(platform->poll_fd.fd, SD_SEND); | ||
| int poll_rv = WSAPoll(&platform->poll_fd, 1, 10000); | ||
| if (0 < poll_rv) | ||
| { | ||
| char dummy; | ||
| while (recv(platform->poll_fd.fd, &dummy, sizeof(dummy), 0) > 0) {}; | ||
| } | ||
| if (0 == closesocket(platform->poll_fd.fd)) | ||
| { | ||
| rv = (0 == WSACleanup()); | ||
| } | ||
| } | ||
| return rv; |
There was a problem hiding this comment.
Same comments as in Linux case.
pablogs9
force-pushed
the
graceful_shutdown
branch
from
ff4aa80
to
06996e1
Compare
|
Build status:
|
This is similar to the third point mentioned in my new PR within the Agent repository.
This revolves around the fact that send() does not guarantee that the receiver will receive all data. By waiting for the other side (the agent in this case) to also close the connection, we can be sure that all data that should have been received, was received.
This also solves another problem: under such a condition, calling close() on the socket() will result in the remote end sending a TCP RST in response to the FIN-ACK.